gc: default-on evacuating young-gen scavenge (moving minor at precise safepoints) - #7019
Conversation
… safepoints) Route nursery pressure to an evacuating (moving/copying) minor GC that runs at precise-root safepoints instead of the non-moving conservative minor, bounding RSS by the nursery size. Previously flag-gated (PERRY_GC_MOVING_LOOP_POLLS=1); now default-on with a kill switch (PERRY_GC_MOVING_LOOP_POLLS=0). Codegen emits a GC loop-back-edge poll (js_gc_loop_safepoint) only for allocating loops (body_may_allocate gate), so numeric/vectorized loops stay call-free. At a back-edge every live heap value is a named shadow-stack local, so the copying minor MOVES survivors precisely (O(live), no sweep). Runtime: - policy.rs: alloc-point nursery triggers DEFER to the next precise safepoint under moving mode; nursery cap via PERRY_GC_SCAVENGE_NURSERY_MB (default 16MB). - copying.rs: rewrite_raw_addr follows the FORWARDED chain in agreement with the verifier so address-keyed metadata (shape table) is rekeyed correctly; eager malloc-registry build under moving mode. - map.rs: Map/Set side-allocation registry re-key on move. Codegen: - pointer_locals.rs: Generic/StringLiteral/TypeVar locals now get precise shadow-stack slots (Map/Set/generic locals were invisible to the moving scan). - loop_purity.rs: body_may_allocate gate. - loops.rs: default-on poll emission. Validated under default (no flags): churn 258->22 MiB, broad3 verifier-clean, numeric no regression (0.24s), 0 fallbacks, VERIFY_EVACUATION clean. Claude Code 2.1.112 doctor 352->169 MiB (node parity) under the moving GC; default re-confirmation in progress. Claude-Session: https://claude.ai/code/session_012GpC1dfiL72KmNcNJdBVm2
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThe PR adds allocation-aware loop safepoints, enables moving-loop polls by default, introduces scavenge configuration and nursery trigger routing, follows copying-GC forwarding chains, expands pointer-root classification, and adjusts moved Map registry handling. ChangesMoving GC integration
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR PerryTS#7019 made the evacuating young-gen moving GC default-on, which changes the effective arena trigger (16 MiB nursery cap vs 128 MiB device ceiling) and routes alloc-point nursery pressure to a deferred precise safepoint instead of an inline budgeted/direct minor. 29 gc:: unit tests assert the old (budgeted/direct, 128 MiB-ceiling) mechanics, which now live behind the PERRY_GC_MOVING_LOOP_POLLS=0 kill switch. The mode gate caches in a process-wide OnceLock, so tests cannot switch mode per-case via env. Fix (no correctness assertion weakened): - policy.rs: add a #[cfg(test)]-only thread-local override for gc_moving_loop_polls_enabled() + an RAII force_legacy_gc_pacing() guard (mirrors the existing gc_trace_enabled test hook). Zero effect in release. - 27 pacer/rooting/copying tests: pin legacy pacing (additive one-liner); they keep asserting exactly what they did, in the mode that mechanism now lives behind the kill switch. (One, test_arena_debt_measured_against_ effective_trigger_not_raw_cell, was added on main after this branch's base and caught by re-testing against live main.) - triggers::test_effective_arena_trigger_respects_armed_values: made mode-aware so it asserts the NEW nursery-cap value (16 MiB) under the default and the device ceiling under the kill switch. gc:: tests: 499 passed / 0 failed under BOTH default and PERRY_GC_MOVING_LOOP_POLLS=0 (verified against live main). Rooting soundness under moving is separately verified (VERIFY_EVACUATION clean; Claude Code doctor evacuated 253552 objects clean). Claude-Session: https://claude.ai/code/session_012GpC1dfiL72KmNcNJdBVm2
…7020) PR #7019 made the evacuating young-gen moving GC default-on, which changes the effective arena trigger (16 MiB nursery cap vs 128 MiB device ceiling) and routes alloc-point nursery pressure to a deferred precise safepoint instead of an inline budgeted/direct minor. 29 gc:: unit tests assert the old (budgeted/direct, 128 MiB-ceiling) mechanics, which now live behind the PERRY_GC_MOVING_LOOP_POLLS=0 kill switch. The mode gate caches in a process-wide OnceLock, so tests cannot switch mode per-case via env. Fix (no correctness assertion weakened): - policy.rs: add a #[cfg(test)]-only thread-local override for gc_moving_loop_polls_enabled() + an RAII force_legacy_gc_pacing() guard (mirrors the existing gc_trace_enabled test hook). Zero effect in release. - 27 pacer/rooting/copying tests: pin legacy pacing (additive one-liner); they keep asserting exactly what they did, in the mode that mechanism now lives behind the kill switch. (One, test_arena_debt_measured_against_ effective_trigger_not_raw_cell, was added on main after this branch's base and caught by re-testing against live main.) - triggers::test_effective_arena_trigger_respects_armed_values: made mode-aware so it asserts the NEW nursery-cap value (16 MiB) under the default and the device ceiling under the kill switch. gc:: tests: 499 passed / 0 failed under BOTH default and PERRY_GC_MOVING_LOOP_POLLS=0 (verified against live main). Rooting soundness under moving is separately verified (VERIFY_EVACUATION clean; Claude Code doctor evacuated 253552 objects clean). Claude-Session: https://claude.ai/code/session_012GpC1dfiL72KmNcNJdBVm2 Co-authored-by: Ralph Küpper <ralph@skelpo.com>
…ix (#7025) (#7040) The harness derived its moved= liveness evidence by summing moved_objects= (C4b evacuation inside the mark-sweep collector) with [gc-copy-minor] ran copied_objects= (the copying young-gen minor #7019 made default-on). Those are different collectors, and summing them let a requires=move cell go green on relocation the arm was not testing. Cell evidence is now 'cycles=N evacuated=X scavenged=Y'; the per-arm liveness summary gains a 'copy-minor n/N' column next to 'moved-objects n/N'. An arm showing healthy moved-objects with copy-minor 0/N did not run the path it exists to exercise -- the shape #7024 describes, which summing hid. Verdicts are unchanged by construction: moved is still evacuated + scavenged and feeds the same predicate, so results stay comparable to the recorded baselines. A stricter predicate is a follow-up gated on #7024, since under --pressure the copying minor currently never runs. Closes #7025. Co-authored-by: Ralph Küpper <ralph@skelpo.com>
…ow (#7022) (#7050) * fix(gc): run the allocation-point GC trigger outside the &mut Arena borrow (#7022) `Arena::alloc(&mut self, ..)` called `gc_check_trigger()` from inside its own borrow. A collection allocates into the arenas — promotion and C4b evacuation call `arena_alloc_gc_old`, an evacuating minor (#7019) fills a survivor semispace — and either can reach `Arena::install_fresh_block` → `self.blocks.push(..)` on the SAME arena the allocating frame is holding. The Vec growth frees the buffer the outer frame goes on to index, and `&mut` carries `noalias`, so the outer frame may also have cached `blocks.ptr`/`len` across the call. Measured on the #7022 reproducer: `install_fresh_block gen=Old space=Old` fires while `Arena::alloc` on the old arena holds the borrow, 204 times in one run, and `self.blocks`'s length changes underneath it. The crashing stack is exactly that shape — `js_array_grow` → `arena_alloc_gc_old` → `Arena::alloc` → `gc_check_trigger` → full mark-sweep → `ValidPointerSetBuilder::step` → `RawVec::grow_one` → `_mi_theap_realloc_zero`, faulting on a corrupt mimalloc heap. `arena_cell_alloc(*mut Arena, ..)` is the new collecting entry point: current block under a borrow that ends with the statement, `gc_check_trigger()` with no arena borrow live, then a fresh borrow for the slow path. `Arena::alloc` is now collection-free, and the five thread-local entry points route through the new one. The two that also touch `INLINE_STATE` keep those borrows short for the same reason: `resync_inline_to_current` mutates `INLINE_STATE` from inside the collection. Same commit, same compiler, only the four arena files differing; `test_gap_repsel_gc_stress` under `PERRY_GC_HEAP_LIMIT=8 PERRY_GC_INCREMENTAL=0 PERRY_CONSERVATIVE_STACK_SCAN=off`, auto-optimized release binaries: 139 x 20 before / 0 x 20 after at the default 16 MB nursery cap, clean across a 13-cap sweep, and 5/5 -> 0/5 on the force-evacuate, force+verify and from-space-scan arms. Claude-Session: https://claude.ai/code/session_01G4k1vE6PVb53m2dRZ2aDtv * docs(changelog): #7050 allocation-point GC trigger outside the arena borrow Claude-Session: https://claude.ai/code/session_01G4k1vE6PVb53m2dRZ2aDtv * docs(changelog): record the #7050 matrix delta (PASS 302->324, FAIL 46->24) Claude-Session: https://claude.ai/code/session_01G4k1vE6PVb53m2dRZ2aDtv --------- Co-authored-by: Ralph Küpper <ralph3@skelpo.com>
…ape (#7034 §4) `Ptr<Shape>` promoted ZERO locals on batch.ts, the object/property-heavy workload the representation exists for: rule 2 (containment) listed `return` as an outright disqualifier and every real record escapes its producing scope, so the proof died at the first escape. `PERRY_PTR_SHAPE_LOCALS=0` vs default produced an identical `__text` — there was nothing to switch off. Producer side: `return <the local>` no longer disqualifies. A `return` is a terminator — every use of the local in that body either precedes it on that path or is unreachable from it, the sole exception being a `finally` block, which still runs before the caller resumes and whose uses the same walk checks anyway. Only the bare form is exempt; a `return` inside a nested closure body is not, because that value escapes at an unbounded later time. Caller side (new collectors/ptr_shape_returns.rs): a module function whose every return path hands back a freshly allocated, unaliased object of one class carries a return-shape fact, and a direct call to it is rule-1 provenance of `new C(...)` strength. Freshness is discharged by re-running the full Phase 3b proof over the producer's body, not by a weaker approximation of it. No ABI change, no cloning, no cross-call-site agreement. Measured: batch.ts 0 -> 2 promoted locals; app-patterns kernels 2 -> 4; benchmarks/suite unchanged at 4. The #7104 census reports the same delta and its batch ptr-shape floor is ratcheted 0 -> 2, so the gain is now gated. The A/B stops being vacuous: __text moves 1,532 bytes between the arms and guard-gate volatile loads drop 26 -> 22. Size only; nothing was timed (the box was under load 40-135 throughout). A call-seeded candidate never claims numeric_fields: the producer's stores are outside the caller's region, so no exhaustive-reachable-store proof is available. Same stand-down, same reason, as collectors/proven_this.rs. GC: no new site holds an object pointer. The caller's binding is the same NaN-boxed slot it always was, shadow-bound by collect_pointer_typed_locals / js_shadow_slot_bind; verified in the IR that the returned register is stored and bound with no intervening safepoint, and that each access re-derives the pointer from the slot. TaPtr's callee-side no-bind shortcut is not copied — it is sound only for non-movable typed-array storage (#6990, #7019). New guard: a producer annotated with a definitely-non-pointer return type would cost the caller's binding its shadow slot, so it carries no fact.
…ape (#7034 §4) `Ptr<Shape>` promoted ZERO locals on batch.ts, the object/property-heavy workload the representation exists for: rule 2 (containment) listed `return` as an outright disqualifier and every real record escapes its producing scope, so the proof died at the first escape. `PERRY_PTR_SHAPE_LOCALS=0` vs default produced an identical `__text` — there was nothing to switch off. Producer side: `return <the local>` no longer disqualifies. A `return` is a terminator — every use of the local in that body either precedes it on that path or is unreachable from it, the sole exception being a `finally` block, which still runs before the caller resumes and whose uses the same walk checks anyway. Only the bare form is exempt; a `return` inside a nested closure body is not, because that value escapes at an unbounded later time. Caller side (new collectors/ptr_shape_returns.rs): a module function whose every return path hands back a freshly allocated, unaliased object of one class carries a return-shape fact, and a direct call to it is rule-1 provenance of `new C(...)` strength. Freshness is discharged by re-running the full Phase 3b proof over the producer's body, not by a weaker approximation of it. No ABI change, no cloning, no cross-call-site agreement. Measured: batch.ts 0 -> 2 promoted locals; app-patterns kernels 2 -> 4; benchmarks/suite unchanged at 4. The #7104 census reports the same delta and its batch ptr-shape floor is ratcheted 0 -> 2, so the gain is now gated. The A/B stops being vacuous: __text moves 1,532 bytes between the arms and guard-gate volatile loads drop 26 -> 22. Size only; nothing was timed (the box was under load 40-135 throughout). A call-seeded candidate never claims numeric_fields: the producer's stores are outside the caller's region, so no exhaustive-reachable-store proof is available. Same stand-down, same reason, as collectors/proven_this.rs. GC: no new site holds an object pointer. The caller's binding is the same NaN-boxed slot it always was, shadow-bound by collect_pointer_typed_locals / js_shadow_slot_bind; verified in the IR that the returned register is stored and bound with no intervening safepoint, and that each access re-derives the pointer from the slot. TaPtr's callee-side no-bind shortcut is not copied — it is sound only for non-movable typed-array storage (#6990, #7019). New guard: a producer annotated with a definitely-non-pointer return type would cost the caller's binding its shadow slot, so it carries no fact. That check calls pointer_locals' own predicate, which this change hoists to module scope so there is exactly one definition — a copy drifting by one Type variant would leave a value unrooted there while proven movable here.
…ape (#7034 §4) `Ptr<Shape>` promoted ZERO locals on batch.ts, the object/property-heavy workload the representation exists for: rule 2 (containment) listed `return` as an outright disqualifier and every real record escapes its producing scope, so the proof died at the first escape. `PERRY_PTR_SHAPE_LOCALS=0` vs default produced an identical `__text` — there was nothing to switch off. Producer side: `return <the local>` no longer disqualifies. A `return` is a terminator — every use of the local in that body either precedes it on that path or is unreachable from it, the sole exception being a `finally` block, which still runs before the caller resumes and whose uses the same walk checks anyway. Only the bare form is exempt; a `return` inside a nested closure body is not, because that value escapes at an unbounded later time. Caller side (new collectors/ptr_shape_returns.rs): a module function whose every return path hands back a freshly allocated, unaliased object of one class carries a return-shape fact, and a direct call to it is rule-1 provenance of `new C(...)` strength. Freshness is discharged by re-running the full Phase 3b proof over the producer's body, not by a weaker approximation of it. No ABI change, no cloning, no cross-call-site agreement. Measured: batch.ts 0 -> 2 promoted locals; app-patterns kernels 2 -> 4; benchmarks/suite unchanged at 4. The #7104 census reports the same delta and its batch ptr-shape floor is ratcheted 0 -> 2, so the gain is now gated. The A/B stops being vacuous: __text moves 1,532 bytes between the arms and guard-gate volatile loads drop 26 -> 22. Size only; nothing was timed (the box was under load 40-135 throughout). A call-seeded candidate never claims numeric_fields: the producer's stores are outside the caller's region, so no exhaustive-reachable-store proof is available. Same stand-down, same reason, as collectors/proven_this.rs. GC: no new site holds an object pointer. The caller's binding is the same NaN-boxed slot it always was, shadow-bound by collect_pointer_typed_locals / js_shadow_slot_bind; verified in the IR that the returned register is stored and bound with no intervening safepoint, and that each access re-derives the pointer from the slot. TaPtr's callee-side no-bind shortcut is not copied — it is sound only for non-movable typed-array storage (#6990, #7019). New guard: a producer annotated with a definitely-non-pointer return type would cost the caller's binding its shadow slot, so it carries no fact. That check calls pointer_locals' own predicate, which this change hoists to module scope so there is exactly one definition — a copy drifting by one Type variant would leave a value unrooted there while proven movable here.
…ape (#7034 §4) (#7107) `Ptr<Shape>` promoted ZERO locals on batch.ts, the object/property-heavy workload the representation exists for: rule 2 (containment) listed `return` as an outright disqualifier and every real record escapes its producing scope, so the proof died at the first escape. `PERRY_PTR_SHAPE_LOCALS=0` vs default produced an identical `__text` — there was nothing to switch off. Producer side: `return <the local>` no longer disqualifies. A `return` is a terminator — every use of the local in that body either precedes it on that path or is unreachable from it, the sole exception being a `finally` block, which still runs before the caller resumes and whose uses the same walk checks anyway. Only the bare form is exempt; a `return` inside a nested closure body is not, because that value escapes at an unbounded later time. Caller side (new collectors/ptr_shape_returns.rs): a module function whose every return path hands back a freshly allocated, unaliased object of one class carries a return-shape fact, and a direct call to it is rule-1 provenance of `new C(...)` strength. Freshness is discharged by re-running the full Phase 3b proof over the producer's body, not by a weaker approximation of it. No ABI change, no cloning, no cross-call-site agreement. Measured: batch.ts 0 -> 2 promoted locals; app-patterns kernels 2 -> 4; benchmarks/suite unchanged at 4. The #7104 census reports the same delta and its batch ptr-shape floor is ratcheted 0 -> 2, so the gain is now gated. The A/B stops being vacuous: __text moves 1,532 bytes between the arms and guard-gate volatile loads drop 26 -> 22. Size only; nothing was timed (the box was under load 40-135 throughout). A call-seeded candidate never claims numeric_fields: the producer's stores are outside the caller's region, so no exhaustive-reachable-store proof is available. Same stand-down, same reason, as collectors/proven_this.rs. GC: no new site holds an object pointer. The caller's binding is the same NaN-boxed slot it always was, shadow-bound by collect_pointer_typed_locals / js_shadow_slot_bind; verified in the IR that the returned register is stored and bound with no intervening safepoint, and that each access re-derives the pointer from the slot. TaPtr's callee-side no-bind shortcut is not copied — it is sound only for non-movable typed-array storage (#6990, #7019). New guard: a producer annotated with a definitely-non-pointer return type would cost the caller's binding its shadow slot, so it carries no fact. That check calls pointer_locals' own predicate, which this change hoists to module scope so there is exactly one definition — a copy drifting by one Type variant would leave a value unrooted there while proven movable here. Co-authored-by: Ralph Küpper <ralph@skelpo.com>
* perf(transform): inline safe cross-module function graphs
* perf(array): reuse resolved headers across indexed stores
* perf(array): split dynamic canonical read keys
* perf(method): guard synthetic-arguments direct calls
* perf(method): scalarize length-only arguments bundles
* perf(array): call captureless some callbacks directly
* perf(method): inline bounded tiny allocation kernels
* perf(inline): optimize functions inside candidate methods
* perf(for-of): preserve Map entry types in function bodies
* perf(array): trust validated rooted iterator headers
* perf(array): establish element shape proofs on demand
* perf(array): reuse dynamic all-pointer append proofs
* perf(property): inline dynamic collection size reads
* perf(compare): inline exact three-byte literal equality
* perf(descriptors): index descriptors by owner instead of scanning every entry
Three hot paths answered "what does THIS owner have?" by walking every
descriptor in the process and filtering on the owner address:
* js_object_keys' array branch, twice (enumeration.rs) — a full
property_descriptors walk per enumeration, just to decide whether a
per-index enumerable check was needed;
* accessor_descriptor_keys_for_obj, on the own-keys path;
* transfer_descriptor_owner, on every ArrayHeader growth;
* scan_descriptor_roots_mut, on EVERY GC cycle — so since the moving
young-gen scavenge became default (#7019) this was a per-collection
tax proportional to the whole program's descriptor count rather than
to what actually moved.
Profiling `claude -p` put 46.6% of main-thread samples in
shapes/descriptors, with a HashMap Keys iteration the single hottest
self-time entry by 4x over anything else.
DescriptorTables now carries attr_keys_by_owner / accessor_keys_by_owner
mirroring the two (owner, key) maps, so each of those becomes a lookup.
The maps stay authoritative; the index is a mirror, and the tests assert
that invariant directly (index == what a full scan would return) across
install, redefine, delete, bulk-clear and owner transfer, because the
failure mode of a mirror is silent drift, not a crash.
Also fixes a pre-existing correctness bug the new tests caught:
transfer_descriptor_owner moved descriptors to the new address but never
carried the per-object Bloom summary. A freshly grown array has a null
meta, for which owner_may_have_descriptor_entries answers false
AUTHORITATIVELY — so after an array grew, Object.keys and
getOwnPropertyDescriptor silently lost every accessor it had. That was
equally true before this change: the gate sat in front of the old scan,
so the scan never ran for the new owner.
* changelog: add fragment for #8875
* ci: clear the lint gates for #8872
The `lint` job failed on four gates that the PR's own changes tripped:
- changelog: add the `changelog.d/8872-*` fragment for the crates/ changes.
- file size: `array/indexing.rs` reached 2,024 lines after the resolved-store
work; move the transactional `js_array_numeric_range_add*` kernel (a block
with no raw-handle or address-classification debt, so no per-module ratchet
ceiling moves) into `array/numeric_range.rs`.
- local-binding-type audit: classify the synthetic `arguments.length` marker
read in `property_get.rs::lower` (runtime-validated: the marker type exists
only in direct-call-only clones whose caller materialized the count).
- GC store-site inventory: register `store_array_slot_resolved` as a
chain-verified discharge helper for the three BARRIERED markers that now
lean on it, mark its own resolved slot write, and pin the second `apush`
codegen marker (the unconditional element store inside
`emit_dynamic_pointer_push_store`, barriered by the same stem) with the
self-test tree updated to match.
Every step of the lint job was replayed locally, including the raw-handle and
unrooted-local ratchets against the merge base d354443.
Claude-Session: https://claude.ai/code/session_01FUvFrRNZyc5qknBiJbYbby
* style: cargo fmt (rustfmt import wrapping after the new re-export)
* fix(runtime): match Node fs readFile prototype
* chore: name r23 changelog for PR
---------
Co-authored-by: Ralph Küpper <ralph@skelpo.com>
Co-authored-by: Ralph Küpper <ralph3@skelpo.com>
What
Makes the evacuating young-gen scavenge (moving/copying minor GC at precise-root safepoints) default-on. Nursery pressure is routed to a copying minor that MOVES survivors instead of the non-moving conservative minor, so RSS is bounded by the nursery size rather than leaking linearly under allocation churn.
Previously flag-gated behind
PERRY_GC_MOVING_LOOP_POLLS=1. Now default-on; kill switch isPERRY_GC_MOVING_LOOP_POLLS=0.How
js_gc_loop_safepoint) only for allocating loops (body_may_allocategate) — numeric/vectorized loops stay call-free (no perf regression). At a back-edge every live heap value is a named shadow-stack local, so the copying minor has precise, rewritable roots and moves survivors in O(live) with no sweep.rewrite_raw_addrfollows the FORWARDED chain in agreement with the verifier, so address-keyed metadata (shape table) is rekeyed correctly after a move (this was the last soundness gap).Generic/StringLiteral/TypeVarlocals now get shadow-stack slots — Map/Set/generic locals were previously invisible to the moving scan.Files (+334 / −35)
Runtime:
gc/policy.rs,gc/copying.rs,gc/mod.rs,map.rsCodegen:
stmt/loops.rs,loop_purity.rs,collectors/pointer_locals.rsValidation
Under default (no flags):
body_may_allocategate holds)doctor352 → 169 MiB (node parity 168) under the moving GC — default re-confirmation in progressFinal cleaned-lib rebuild + churn/broad3 re-validation + default
doctorare being run post-merge-open (per request, to unblock downstream work now). Will post numbers here.Note for downstream
This is the base for the statepoints / stack-map root migration — it keeps all the evacuation machinery and only swaps the root source (shadow-stack scan → compiler-emitted stack maps +
gc.relocate), which additionally attacks keystroke latency and per-alloc throughput. Branch offgc/moving-scavenge-default-on(or this commit90f370380).Base note
Branch is based on
83a6767ff(a clean ancestor ofmain), so the diff here is exactly the 7 GC files.https://claude.ai/code/session_012GpC1dfiL72KmNcNJdBVm2
Summary by CodeRabbit
Performance
Bug Fixes